-
Notifications
You must be signed in to change notification settings - Fork 623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(semver): prepare for noUncheckedIndexedAccess
#4354
Conversation
semver/parse_range.ts
Outdated
@@ -5,8 +5,8 @@ import { OPERATOR_XRANGE_REGEXP, XRANGE } from "./_shared.ts"; | |||
import { parseComparator } from "./_parse_comparator.ts"; | |||
import { parseBuild, parsePrerelease } from "./_shared.ts"; | |||
|
|||
function isWildcard(id: string): boolean { | |||
return !id || id.toLowerCase() === "x" || id === "*"; | |||
function isWildcard(id: string | undefined): boolean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the optional ?
assertion for option arguments instead of undefined
union types. Ditto to other instances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, but ?
is only available for functions whose required parameters don't follow an optional one. Meaning, I can change it in isWildcard()
, but not in parseNumber()
(_shared.ts
)
13a79ef
to
56f193f
Compare
changed to using |
semver/_shared.ts
Outdated
export function parseNumber(input: string | undefined, errorMessage: string) { | ||
if (input === undefined) { | ||
return 0; | ||
} | ||
const number = Number(input); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems wrong. Instead, I think we should use non-null assertions for the values we pass to the input
argument. Even if the input
is null
, the function should fail, which is what we want.
8ea975c
to
4802477
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you for this.
noUncheckedIndexedAccess
ref #4040